Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🎉 Source Okta: add User_Role_Assignments and Group_Role_Assignments #14556

Merged
merged 5 commits into from
Jul 18, 2022

Conversation

YiyangLi
Copy link
Contributor

@YiyangLi YiyangLi commented Jul 9, 2022

What

How

Describe the solution

Recommended reading order

  1. source.python
  2. source_okta/schemas/shared/shared-role.json

🚨 User Impact 🚨

No really, new API streams

Pre-merge Checklist

Expand the relevant checklist and delete the others.

New Connector

Community member or Airbyter

  • Community member? Grant edit access to maintainers (instructions)
  • Secrets in the connector's spec are annotated with airbyte_secret
  • Unit & integration tests added and passing. Community members, please provide proof of success locally e.g: screenshot or copy-paste unit, integration, and acceptance test output. To run acceptance tests for a Python connector, follow instructions in the README. For java connectors run ./gradlew :airbyte-integrations:connectors:<name>:integrationTest.
  • Code reviews completed
  • Documentation updated
    • Connector's README.md
    • Connector's bootstrap.md. See description and examples
    • docs/integrations/<source or destination>/<name>.md including changelog. See changelog example
    • docs/integrations/README.md
    • airbyte-integrations/builds.md
  • PR name follows PR naming conventions

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

  • Create a non-forked branch based on this PR and test the below items on it
  • Build is successful
  • If new credentials are required for use in CI, add them to GSM. Instructions.
  • /test connector=connectors/<name> command is passing
  • New Connector version released on Dockerhub by running the /publish command described here
  • After the connector is published, connector added to connector index as described here
  • Seed specs have been re-generated by building the platform and committing the changes to the seed spec files, as described here
Updating a connector

Community member or Airbyter

  • Grant edit access to maintainers (instructions)
  • Secrets in the connector's spec are annotated with airbyte_secret
  • Unit & integration tests added and passing. Community members, please provide proof of success locally e.g: screenshot or copy-paste unit, integration, and acceptance test output. To run acceptance tests for a Python connector, follow instructions in the README. For java connectors run ./gradlew :airbyte-integrations:connectors:<name>:integrationTest.
  • Code reviews completed
  • Documentation updated
    • Connector's README.md
    • Connector's bootstrap.md. See description and examples
    • Changelog updated in docs/integrations/<source or destination>/<name>.md including changelog. See changelog example
  • PR name follows PR naming conventions

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

  • Create a non-forked branch based on this PR and test the below items on it
  • Build is successful
  • If new credentials are required for use in CI, add them to GSM. Instructions.
  • /test connector=connectors/<name> command is passing
  • New Connector version released on Dockerhub and connector version bumped by running the /publish command described here
Connector Generator
  • Issue acceptance criteria met
  • PR name follows PR naming conventions
  • If adding a new generator, add it to the list of scaffold modules being tested
  • The generator test modules (all connectors with -scaffold in their name) have been updated with the latest scaffold by running ./gradlew :airbyte-integrations:connector-templates:generator:testScaffoldTemplates then checking in your changes
  • Documentation which references the generator is updated as needed

Tests

Unit

Put your unit tests output here.

Integration

Put your integration tests output here.

Acceptance
collecting ... 
 airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_core.py::TestSpec.test_config_match_spec[inputs0] ✓                                                                                                                         4% ▌         
 airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_core.py::TestSpec.test_match_expected[inputs0] ✓                                                                                                                            8% ▉         
 airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_core.py::TestSpec.test_docker_env[inputs0] ✓                                                                                                                               12% █▍        
 airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_core.py::TestSpec.test_oneof_usage[inputs0] ✓                                                                                                                              17% █▋        
 airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_core.py::TestSpec.test_required[inputs0] ✓                                                                                                                                 21% ██▏       
 airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_core.py::TestSpec.test_optional[inputs0] ✓                                                                                                                                 25% ██▌       
 airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_core.py::TestSpec.test_has_secret[inputs0] ✓                                                                                                                               29% ██▉       
 airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_core.py::TestSpec.test_secret_never_in_the_output[inputs0] ✓                                                                                                               33% ███▍      
 airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_core.py::TestSpec.test_defined_refs_exist_in_json_spec_file[inputs0] ✓                                                                                                     38% ███▊      
 airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_core.py::TestSpec.test_oauth_flow_parameters[inputs0] ✓                                                                                                                    42% ████▎     
 airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_core.py::TestConnection.test_check[inputs0] ✓                                                                                                                              46% ████▋     
 airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_core.py::TestConnection.test_check[inputs1] ✓                                                                                                                              50% █████     
 airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_core.py::TestDiscovery.test_discover[inputs0] ✓                                                                                                                            54% █████▌    
 airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_core.py::TestDiscovery.test_defined_cursors_exist_in_schema[inputs0] ✓                                                                                                     58% █████▉    
 airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_core.py::TestDiscovery.test_defined_refs_exist_in_schema[inputs0] ✓                                                                                                        62% ██████▍   
 airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_core.py::TestDiscovery.test_defined_keyword_exist_in_schema[inputs0-allOf] ✓                                                                                               67% ██████▋   
 airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_core.py::TestDiscovery.test_defined_keyword_exist_in_schema[inputs0-not] ✓                                                                                                 71% ███████▏  
 airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_core.py::TestDiscovery.test_primary_keys_exist_in_schema[inputs0] ✓                                                                                                        75% ███████▌  
 airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_core.py::TestBasicRead.test_read[inputs0] ✓                                                                                                                                79% ███████▉  {"type": "LOG", "log": {"level": "ERROR", "message": "Docker container was failed, code 1, error:\n{\"type\": \"TRACE\", \"trace\": {\"type\": \"ERROR\", \"emitted_at\": 1657406797238.836, \"error\": {\"message\": \"Something went wrong in the connector. See the logs for more details.\", \"internal_message\": \"2 validation errors for ConfiguredAirbyteCatalog\\nstreams -> 0 -> sync_mode\\n  value is not a valid enumeration member; permitted: 'full_refresh', 'incremental' (type=type_error.enum; enum_values=[<SyncMode.full_refresh: 'full_refresh'>, <SyncMode.incremental: 'incremental'>])\\nstreams -> 0 -> destination_sync_mode\\n  value is not a valid enumeration member; permitted: 'append', 'overwrite', 'append_dedup' (type=type_error.enum; enum_values=[<DestinationSyncMode.append: 'append'>, <DestinationSyncMode.overwrite: 'overwrite'>, <DestinationSyncMode.append_dedup: 'append_dedup'>])\", \"stack_trace\": \"Traceback (most recent call last):\\n  File \\\"/airbyte/integration_code/main.py\\\", line 13, in <module>\\n    launch(source, sys.argv[1:])\\n  File \\\"/usr/local/lib/python3.9/site-packages/airbyte_cdk/entrypoint.py\\\", line 116, in launch\\n    for message in source_entrypoint.run(parsed_args):\\n  File \\\"/usr/local/lib/python3.9/site-packages/airbyte_cdk/entrypoint.py\\\", line 104, in run\\n    config_catalog = self.source.read_catalog(parsed_args.catalog)\\n  File \\\"/usr/local/lib/python3.9/site-packages/airbyte_cdk/sources/source.py\\\", line 54, in read_catalog\\n    return ConfiguredAirbyteCatalog.parse_obj(self.read_config(catalog_path))\\n  File \\\"/usr/local/lib/python3.9/site-packages/pydantic/main.py\\\", line 521, in parse_obj\\n    return cls(**obj)\\n  File \\\"/usr/local/lib/python3.9/site-packages/pydantic/main.py\\\", line 341, in __init__\\n    raise validation_error\\npydantic.error_wrappers.ValidationError: 2 validation errors for ConfiguredAirbyteCatalog\\nstreams -> 0 -> sync_mode\\n  value is not a valid enumeration member; permitted: 'full_refresh', 'incremental' (type=type_error.enum; enum_values=[<SyncMode.full_refresh: 'full_refresh'>, <SyncMode.incremental: 'incremental'>])\\nstreams -> 0 -> destination_sync_mode\\n  value is not a valid enumeration member; permitted: 'append', 'overwrite', 'append_dedup' (type=type_error.enum; enum_values=[<DestinationSyncMode.append: 'append'>, <DestinationSyncMode.overwrite: 'overwrite'>, <DestinationSyncMode.append_dedup: 'append_dedup'>])\\n\", \"failure_type\": \"system_error\"}}}\n"}}

 airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_core.py::TestBasicRead.test_airbyte_trace_message_on_failure[inputs0] ✓                                                                                                    83% ████████▍ 
 airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_full_refresh.py::TestFullRefresh.test_sequential_reads[inputs0] ✓                                                                                                          88% ████████▊ 
 airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_incremental.py::TestIncremental.test_two_sequential_reads[inputs0] ✓                                                                                                       92% █████████▎
 airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_incremental.py::TestIncremental.test_read_sequential_slices[inputs0] ✓                                                                                                     96% █████████▋
 airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_incremental.py::TestIncremental.test_state_with_abnormally_large_values[inputs0] ✓                                                                                        100% ██████████
{"type": "LOG", "log": {"level": "INFO", "message": "/Users/bartdev/Playground/airbyte/airbyte-integrations/connectors/source-okta - SAT run - dd197963877de218670a7a84b2a6de5851cf9c47 - PASSED"}}

============================================================================================================================== warnings summary ==============================================================================================================================
airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_core.py: 17 warnings
airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_full_refresh.py: 1 warning
airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_incremental.py: 3 warnings
  /Users/bartdev/Playground/airbyte/airbyte-integrations/connectors/source-okta/.venv/lib/python3.9/site-packages/docker/utils/utils.py:52: DeprecationWarning: distutils Version classes are deprecated. Use packaging.version instead.
    s1 = StrictVersion(v1)

airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_core.py: 17 warnings
airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_full_refresh.py: 1 warning
airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_incremental.py: 3 warnings
  /Users/bartdev/Playground/airbyte/airbyte-integrations/connectors/source-okta/.venv/lib/python3.9/site-packages/docker/utils/utils.py:53: DeprecationWarning: distutils Version classes are deprecated. Use packaging.version instead.
    s2 = StrictVersion(v2)

-- Docs: https://docs.pytest.org/en/stable/warnings.html

Results (61.69s):

@github-actions github-actions bot added area/connectors Connector related issues area/documentation Improvements or additions to documentation labels Jul 9, 2022
@YiyangLi YiyangLi changed the title 🎉 Add User Role Assignments and Group Role Assignment 🎉 add User_Role_Assignments and Group_Role_Assignments streams Jul 9, 2022
@YiyangLi YiyangLi force-pushed the YL_add-roles branch 5 times, most recently from b291650 to 3a38742 Compare July 9, 2022 23:01
@YiyangLi YiyangLi marked this pull request as ready for review July 9, 2022 23:03
@YiyangLi YiyangLi changed the title 🎉 add User_Role_Assignments and Group_Role_Assignments streams 🎉 Source Okta: add User_Role_Assignments and Group_Role_Assignments Jul 10, 2022
@alafanechere alafanechere added the bounty-M Maintainer program: claimable medium bounty PR label Jul 11, 2022
@YiyangLi YiyangLi mentioned this pull request Jul 12, 2022
10 tasks
@marcosmarxm
Copy link
Member

marcosmarxm commented Jul 12, 2022

/test connector=connectors/source-okta

🕑 connectors/source-okta https://github.com/airbytehq/airbyte/actions/runs/2658053243
❌ connectors/source-okta https://github.com/airbytehq/airbyte/actions/runs/2658053243
🐛 https://gradle.com/s/2dn7benlzhcvm

Build Failed

Test summary info:

=========================== short test summary info ============================
FAILED test_core.py::TestBasicRead::test_read[inputs0] - AssertionError: All ...
======================== 1 failed, 23 passed in 38.19s =========================

@YiyangLi YiyangLi force-pushed the YL_add-roles branch 2 times, most recently from c9fb3f1 to 8a6d5a9 Compare July 13, 2022 02:16
@YiyangLi
Copy link
Contributor Author

To fix streams without records in acceptance test

Hi, I realized the CI reports the following error message. If possible, can you help to assign a role to a group in your test okta org? If so, I don't have to add the stream to empty_streams. I am happy to provide screenshots.

self = <source_acceptance_test.tests.test_core.TestBasicRead object at 0x7efdc7581520>
records = [AirbyteRecordMessage(namespace=None, stream='users', data=***'id': '00umj7d1bEH95JjIE5d6', 'status': 'ACTIVE', 'created...***'lat': 50.0585, 'lon': 19.9342***, 'version': 'V4', 'source': None***]***, 'target': None***, emitted_at=1657644736814), ...]
configured_catalog = ConfiguredAirbyteCatalog(streams=[ConfiguredAirbyteStream(stream=AirbyteStream(name='users', json_schema=***'$id': 'shar...l_refresh'>, cursor_field=None, destination_sync_mode=<DestinationSyncMode.overwrite: 'overwrite'>, primary_key=None)])
allowed_empty_streams = ***'logs'***

    def _validate_empty_streams(self, records, configured_catalog, allowed_empty_streams):
        """
        Only certain streams allowed to be empty
        """
        counter = Counter(record.stream for record in records)
    
        all_streams = set(stream.stream.name for stream in configured_catalog.streams)
        streams_with_records = set(counter.keys())
        streams_without_records = all_streams - streams_with_records
    
        streams_without_records = streams_without_records - allowed_empty_streams
>       assert not streams_without_records, f"All streams should return some records, streams without records: ***streams_without_records***"
E       AssertionError: All streams should return some records, streams without records: ***'group_role_assignments'***
E       assert not ***'group_role_assignments'***

/usr/local/lib/python3.9/site-packages/source_acceptance_test/tests/test_core.py:315: AssertionError

To create a new group role assignment in the okta test org, you may follow the following steps.

  1. create a new group, say test runners. Go to Directory --> Groups. If you don't want to create a new one, you can also pick an old group. Please avoid the group called everyone. Since you really don't want to assign an admin role to EVERYONE.

Screen Shot 2022-07-12 at 7 19 32 PM

it's okay to leave the group empty, without any members.

Screen Shot 2022-07-12 at 7 28 28 PM

  1. Assign an admin role to the group, you could use the Admin roles tab in the group's page
    Screen Shot 2022-07-12 at 7 21 12 PM

Choose a role under the dropdown list, Read only Admin is the one with least permissions. Click on Save.
Screen Shot 2022-07-12 at 7 29 49 PM

  1. You could use the navigation drawer on the left, Security --> Administrators --> Roles tab to assign a role to a group.

Screen Shot 2022-07-12 at 7 22 58 PM

Choose the dropdown, and go to next page to assign it to a group. For our case, it would be test runner, since the group is empty, no users will get extra permissions.

Screen Shot 2022-07-12 at 7 24 16 PM

@marcosmarxm
Copy link
Member

@YiyangLi can you solve the conflicts?

- it's all about okta source connector
- these 2 API streams don't support the sync mode incremental, the api doesn's mention cursor or pagination for these 2 APIs
- add $id to shared schema, rename shared schema
@YiyangLi
Copy link
Contributor Author

Hi, @harshithmullapudi the conflicts have been addressed, can you help run the test again?

@marcosmarxm
Copy link
Member

marcosmarxm commented Jul 14, 2022

/test connector=connectors/source-okta

🕑 connectors/source-okta https://github.com/airbytehq/airbyte/actions/runs/2670642632
❌ connectors/source-okta https://github.com/airbytehq/airbyte/actions/runs/2670642632
🐛 https://gradle.com/s/53cq3xkfy7dgi

Build Failed

Test summary info:

=========================== short test summary info ============================
FAILED test_core.py::TestBasicRead::test_read[inputs0] - AssertionError: All ...
=================== 1 failed, 23 passed in 69.00s (0:01:09) ====================

@YiyangLi
Copy link
Contributor Author

@marcosmarxm I saw the test fails at All streams should return some records, streams without records: ***'group_role_assignments'***

I included the steps to create a group role assignment in an okta org, can you create one and run the test again? FYI, creating a group role assignment on a group without members wouldn't affect your org setup.

@harshithmullapudi
Copy link
Contributor

harshithmullapudi commented Jul 18, 2022

/test connector=connectors/source-okta

🕑 connectors/source-okta https://github.com/airbytehq/airbyte/actions/runs/2689008947
✅ connectors/source-okta https://github.com/airbytehq/airbyte/actions/runs/2689008947
Python tests coverage:

Name                                                 Stmts   Miss  Cover
------------------------------------------------------------------------
source_acceptance_test/utils/__init__.py                 6      0   100%
source_acceptance_test/tests/__init__.py                 4      0   100%
source_acceptance_test/__init__.py                       2      0   100%
source_acceptance_test/tests/test_full_refresh.py       52      2    96%
source_acceptance_test/utils/asserts.py                 37      2    95%
source_acceptance_test/config.py                        77      6    92%
source_acceptance_test/utils/json_schema_helper.py     105     13    88%
source_acceptance_test/tests/test_incremental.py       121     25    79%
source_acceptance_test/utils/common.py                  80     17    79%
source_acceptance_test/tests/test_core.py              294    106    64%
source_acceptance_test/utils/compare.py                 62     23    63%
source_acceptance_test/base.py                          10      4    60%
source_acceptance_test/utils/connector_runner.py       110     48    56%
------------------------------------------------------------------------
TOTAL                                                  960    246    74%
Name                      Stmts   Miss  Cover
---------------------------------------------
source_okta/__init__.py       2      0   100%
source_okta/source.py       147     69    53%
---------------------------------------------
TOTAL                       149     69    54%

Build Passed

Test summary info:

All Passed

@harshithmullapudi
Copy link
Contributor

Thanks @YiyangLi a lot for the directions I was able to get that thing done

@harshithmullapudi
Copy link
Contributor

harshithmullapudi commented Jul 18, 2022

/publish connector=connectors/source-okta

🕑 Publishing the following connectors:
connectors/source-okta
https://github.com/airbytehq/airbyte/actions/runs/2692695431


Connector Did it publish? Were definitions generated?
connectors/source-okta

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

@harshithmullapudi
Copy link
Contributor

harshithmullapudi commented Jul 18, 2022

/publish connector=connectors/source-okta

🕑 Publishing the following connectors:
connectors/source-okta
https://github.com/airbytehq/airbyte/actions/runs/2692712780


Connector Did it publish? Were definitions generated?
connectors/source-okta

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/connectors Connector related issues area/documentation Improvements or additions to documentation bounty bounty-M Maintainer program: claimable medium bounty PR community connectors/source/okta reward-100
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

6 participants